Skip to content

Conversation

@NCBM
Copy link
Contributor

@NCBM NCBM commented Dec 17, 2025

fixes #1627

@github-actions

This comment has been minimized.

@DetachHead
Copy link
Owner

to get auto imports working, you can probably use the same method i used for completions

const importTextEditInfo = this.createAutoImporter(
completionMap,
this.options.lazyEdit
).getTextEditsForAutoImportByFilePath(
{ name: 'override' },
{ name: overrideDecorator.shared.moduleName },
'override',
ImportGroup.BuiltIn,
overrideDecorator.shared.declaration!.uri
);
if (importTextEditInfo.edits) {
additionalTextEdits.push(...importTextEditInfo.edits);
}
additionalTextEdits.push({
range: { start: position, end: position },
replacementText: `@${importTextEditInfo.insertionText}\n${indent}`,
});

@NCBM
Copy link
Contributor Author

NCBM commented Dec 21, 2025

to get auto imports working, you can probably use the same method i used for completions

The AutoImporter does import, however I firstly tried and found that it still does import even when the symbol is already imported - this causes duplicate imports.

Using CompletionProvider is also currently not possible - it can only process the symbol already been in the document.

I'm not sure either duplicate import or letting users do double-fix would be better, or there are better solutions?

@DetachHead
Copy link
Owner

i was able to get it working. i just pushed my changes. needs to be cleaned up a bit though (mainly the duplicated code)

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@NCBM NCBM changed the title add code action to add @override add code action quick-fix to add @override Dec 25, 2025
@NCBM NCBM marked this pull request as ready for review December 25, 2025 12:44
@github-actions

This comment has been minimized.

Copy link
Owner

@DetachHead DetachHead left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the contribution

@github-actions

This comment has been minimized.

@NCBM
Copy link
Contributor Author

NCBM commented Dec 27, 2025

looks like some execution order randomly incorrect (check before parse) on this macos test. maybe related to a new issue or some hard-to-reproduce issues.

private _checkTypes(
fileToCheck: SourceFileInfo,
options?: { chainedByList?: SourceFileInfo[]; skipFileNeededCheck?: boolean }
) {
// For very large programs, we may need to discard the evaluator and
// its cached types to avoid running out of heap space.
this._handleMemoryHighUsage();
return this._logTracker.log(`analyzing: ${fileToCheck.uri}`, (logState) => {
// If the file isn't needed because it was eliminated from the
// transitive closure or deleted, skip the file rather than wasting
// time on it.
if (!this._isFileNeeded(fileToCheck)) {
logState.suppress();
return false;
}
if (!fileToCheck.sourceFile.isCheckingRequired()) {
logState.suppress();
return false;
}
if (!options?.skipFileNeededCheck && !this._shouldCheckFile(fileToCheck)) {
logState.suppress();
return false;
}
// Bind the file if necessary even if we're not going to run the checker.
// disableChecker means disable semantic errors, not syntax errors. We need to bind again
// in order to generate syntax errors.
const boundFile = this._bindFile(
fileToCheck,
undefined,
// If binding is required we want to make sure to bind the file, otherwise
// the sourceFile.check below will fail.
/* skipFileNeededCheck */ fileToCheck.sourceFile.isBindingRequired()
);
if (!this._disableChecker) {
// For ipython, make sure we check all its dependent files first since
// their results can affect this file's result.
const dependentFiles = this._checkDependentFiles(fileToCheck, options?.chainedByList);
if (this._preCheckCallback) {
const parseResults = fileToCheck.sourceFile.getParserOutput();
if (parseResults) {
this._preCheckCallback(parseResults, this._evaluator!);
}
}
if (boundFile) {
fileToCheck.sourceFile.check(
this.configOptions,
this._lookUpImport,
this._importResolver,
this._evaluator!,
dependentFiles
);
}
}

@github-actions
Copy link
Contributor

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

@DetachHead
Copy link
Owner

yeah i've seen that failure before, no idea what causes it but re-running the job usually fixes it. i'll open a separate issue to investigate

@DetachHead DetachHead merged commit a4030c6 into DetachHead:main Dec 27, 2025
18 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

reportImplicitOverride should offer a quick-fix to add @override

2 participants